Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change consolidated balance calculation + small refactor #8908

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

StefanBratanov
Copy link
Contributor

@StefanBratanov StefanBratanov commented Dec 10, 2024

PR Description

  • Disabled ELECTRA ref tests while waiting for v1.5.0-alpha.10 release
  • Main change is in the EpochProcessorElectra
  • The rest is just small nits, rearranging of methods, etc

Fixed Issue(s)

fixes #8891

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@StefanBratanov StefanBratanov changed the title Change consolidating balance calculation + small refactor Change consolidated balance calculation + small refactor Dec 10, 2024
Comment on lines -363 to -365
/*
Implements process_deposit_request from consensus-specs (EIP-6110)
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moving all of these fns does make it a lot harder to determine if there were changes in githubs ui...

@StefanBratanov StefanBratanov force-pushed the 8891_fix branch 2 times, most recently from a9b079f to e99e85b Compare December 11, 2024 15:54
Comment on lines -325 to +336
final UInt64 activeBalance =
stateAccessorsElectra.getActiveBalance(state, pendingConsolidation.getSourceIndex());
// Calculate the consolidated balance
final UInt64 sourceEffectiveBalance =
state
.getBalances()
.get(pendingConsolidation.getSourceIndex())
.get()
.min(sourceValidator.getEffectiveBalance());
// Move active balance to target. Excess balance is withdrawable.
beaconStateMutators.decreaseBalance(
state, pendingConsolidation.getSourceIndex(), activeBalance);
state, pendingConsolidation.getSourceIndex(), sourceEffectiveBalance);
beaconStateMutators.increaseBalance(
state, pendingConsolidation.getTargetIndex(), activeBalance);
state, pendingConsolidation.getTargetIndex(), sourceEffectiveBalance);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it easier for others to review. These are the spec changes:

image

And these changes track those properly 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep next time i think this can be a PR and refactor can be separate for sanity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, noted for next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EIP-7251] Limit consolidating balance by validator.effective_balance
4 participants